Skip to content

Upsert Content database rows by stable key - #2582

Merged
3mdistal merged 15 commits into
mainfrom
codex/content-upsert-key-3192
Aug 4, 2026
Merged

Upsert Content database rows by stable key#2582
3mdistal merged 15 commits into
mainfrom
codex/content-upsert-key-3192

Conversation

@3mdistal

@3mdistal 3mdistal commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Content can create database rows, but it cannot safely project an external record into the same row across retries. add-database-item always creates a fresh identity, so unattended imports and projections can duplicate rows or lose the connection between a source key and the Content item it created.

Concurrent callers also need an unambiguous receipt. Without serialization through the post-commit readback, a later writer can overtake the first and make a successfully committed mutation return an internal error.

Approach

Add one generic, authenticated Content action that upserts a row by a caller-supplied stable string key. The database owns the identity fence; the action owns access checks, transactional row mutation, explicit created, updated, or unchanged receipts, and bounded readback.

The action is source-agnostic and does not invoke Content's built-in AI for deterministic writes.

What changed

  • Add upsert-database-item-by-key, returning a stable item/document identity and exact bounded readback.
  • Add opt-in stable-key claim storage with uniqueness for both (database, key property, key value) and one active value per (database, key property, row).
  • Validate database and row editor access, writable property types, exact database membership, stale claims, legacy duplicates, requested property values, title, and body.
  • Reject system catalog databases and source-managed key/property writes; stable-key identity is available only to ordinary Content databases.
  • Preserve inherited visibility and shares on creation.
  • Retire claims atomically when ordinary property edits change the key, when a row membership is removed, and during property or permanent document/database deletion. Soft deletion keeps the claim for restoration.
  • Keep one database → definition → membership lock order across stable-key upsert, ordinary property writes, source binding, property configuration, and deletion.
  • Serialize the write and exact receipt readback across PostgreSQL workers with a database advisory lock. The lock uses a disposable connection outside the ordinary application pool, and a process-wide gate bounds it to one extra connection per process.
  • Register the action in the Content agent card and parity matrix.

Safety and operations

  • Migrations 78 and 79 are additive. Existing rows are not backfilled and existing add-database-item behavior is unchanged.
  • The action is intentionally not parallel-safe. Stable-key calls are serialized within each process, and PostgreSQL coordinates matching identities across processes.
  • The private lock connection cannot consume the shared application pool and is closed after every receipt, avoiding the small-pool deadlock failure mode.
  • Rollback can remove the action while leaving the additive claim table unused; this change does not rewrite existing Content rows.
  • The migrations use stable unique names, so numeric version overlap on parallel branches remains safe.
  • No hosted Content data was changed, and the existing Content sync-test page was left unchanged.

Product roadmap impact

content_product_impact:
  lane: contract_fulfillment
  features:
    - content.feature.when-this-happens-that-follows
  capabilities:
    - content.agent.action-parity
    - content.api.cms
  record_change: none
  proof:
    - 204 targeted Content database tests on the current head
    - real PostgreSQL migrations and authenticated two-process HTTP acceptance
    - bounded Content action readback and independent concurrency review
  rationale: Adds a shared typed Action and idempotent database-row receipt substrate for authorized agent, automation, and external clients without claiming Feature or Capability completion.

This fulfills part of the roadmap's shared-Action contract. It does not implement Content's Event or Rule model, public client packaging, scheduled automation, or a Source synchronization policy, so no roadmap record changes state in this PR.

Verification

  • 20 focused stable-key tests and 184 broader Content database tests pass on the current head, covering receipt semantics, identity uniqueness, authorization, lifecycle cleanup, source/property transitions, editor reconciliation, and concurrent ordinary writes.
  • Core and Content typechecks pass, and the production Content build completes. The build reports the repository's existing 83 non-fatal doctor findings.
  • Additive-migration, migration-manifest, unscoped-query, generated-artifact, and secret-literal guards pass; formatting and git diff --check are clean.
  • A disposable PostgreSQL 17 database was reduced to the pre-v78 migration state, then the production server applied migrations 78 and 79, recreated all five expected claim indexes, preserved 12 documents, 13 memberships, and five existing property values, and performed an idempotent second boot.
  • Real HTTP access checks returned 401 without authentication and 403 for an authenticated non-owner, with no denied stable-key claims written.
  • Two production server processes sharing PostgreSQL passed conflicting-payload and identical-payload races. A four-request same-key burst also completed without timeout: all responses were 200, all returned one stable item/document identity, and every receipt's readback matched that caller's requested payload.
  • Independent bounded reads through get-content-database (five rows per page) and get-document returned the exact accepted row and body. Direct PostgreSQL verification found exactly one claim, item, document, and key value for each exercised identity.
  • Independent review found no remaining blocking concurrency defect. The durable unit suite remains SQLite-based; the real-PostgreSQL acceptance above covers the advisory-lock branch for this head.

Review focus

  • Do the two uniqueness constraints express the right identity and aliasing boundaries?
  • Does the separate, process-bounded advisory-lock connection provide the right liveness/correctness trade-off?
  • Are claim cleanup and row/property deletion correctly scoped and atomic?
  • Are row-level authorization and inherited sharing consistent with existing Content write actions?

Follow-ups

  • Optional improvement: codify the disposable two-process PostgreSQL acceptance case as a reusable repository test harness.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@3mdistal
3mdistal marked this pull request as ready for review August 2, 2026 18:09
builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

@3mdistal

3mdistal commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the Builder review against 71eb4c160:

  • Migration v78 overlap — no code change: both migrations are named. Core migration bookkeeping applies named migrations by their stable unique names, independently of the legacy numeric watermark. PR Add atomic Content database row migrations #2573 uses content-database-migration-receipts; this PR uses content-database-item-stable-key-claims.
  • Different-key position ties — no code change: position is presentation ordering, not identity. Database reads deterministically tie-break equal positions by createdAt and item ID, and this action is not marked parallelSafe.
  • Favorites membership cleanup — fixed: stable-key upserts now reject every system-role database, including Favorites, Files, and Workspaces, before creating a claim.
  • Concurrent missing-property insert — fixed: existing-row upserts now acquire a database row lock through the exact canonical membership before rereading or inserting property values. Added regression coverage proving one stored value after concurrent requests.

Verification on the repaired head: 88 targeted tests across 15 files, Content typecheck, additive-migration guard, unscoped-query guard, formatting, git diff --check, and independent persistence review.

Hosted separate-worker/PostgreSQL contention remains an explicit deployment acceptance gate.

  • Codex AI

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@3mdistal

3mdistal commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the incremental Builder review on b1f07f4ea:

  • Existing-row body updates — resolved through Content's established external-write contract: the editor intentionally adopts genuinely newer SQL snapshots into the lead Yjs client. The upsert now also locks the exact document row and advances updatedAt monotonically; a future-timestamp regression proves the projected body remains newer. The post-commit refresh signal triggers the canonical refetch/reconcile path.
  • Concurrent database deletion — fixed: upsert, source binding, and trash now share the canonical content_databases row lock. Trash recollects the subtree after acquiring the lock, so an upsert that commits first is included and an upsert that arrives later fails active-database revalidation. A boundary trigger regression injects a late child exactly on lock acquisition and proves it is trashed.
  • Source-managed stable keys — fixed in both directions: upsert rejects an already mapped property before and inside its transaction. Source binding now takes the same database lock and rejects any property with active stable-key claims before mapping or backfill, in one transaction. Reverse-order coverage proves the binding remains unchanged.

Verification on the repaired head: 133 targeted tests across five persistence/editor files, Content typecheck, additive-migration guard, unscoped-query guard, formatting, and git diff --check. Independent review reran 130 tests across the same five files and found no remaining local blocker.

Hosted authenticated invocation, migration application, exact private-row readback, and separate-process/PostgreSQL contention remain explicit acceptance gates before unattended use.

  • Codex AI

builder-io-integration[bot]

This comment was marked as outdated.

@3mdistal

3mdistal commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the latest Builder finding on fb2d75b8c:

  • Concurrent property deletion/configuration — fixed with a shared lock order: stable-key upsert, existing-property configuration, and property deletion now acquire the canonical database row and then the target definition row before touching claims or values.
  • In-transaction definition revalidation: upsert locks every requested definition and verifies its exact ID, type, and system role before creating or updating identity state.
  • Atomic type transitions: changing a property type now removes both old serialized values and stable-key claims in the same transaction before updating the definition.
  • Reverse-order regression: the real configure action races a stable-key upsert; regardless of lock winner, the final new type has no old-type value or stranded claim. A separate lock-boundary regression proves a deleted definition produces no item or claim.

Verification on the repaired head: 135 targeted tests across five persistence/editor files, Content typecheck, additive-migration guard, unscoped-query guard, formatting, and git diff --check. Independent review found no remaining blocker in the shared lifecycle protocol.

Hosted authenticated invocation, migration application, exact private-row readback, and separate-process/PostgreSQL contention remain explicit acceptance gates before unattended use.

  • Codex AI

builder-io-integration[bot]

This comment was marked as outdated.

@3mdistal

3mdistal commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the latest Builder findings on 3094c19cc:

  • Ordinary property writes — fixed: set-document-property now uses the shared database → definition → membership lock order before writing a value or retiring a mismatched claim. A real concurrent set/upsert regression proves the stable value and claim remain aligned for either lock winner.
  • Source binding definition snapshot — fixed: binding now locks and revalidates the exact property definition after the database lock, before claim checks, mapping, or backfill.
  • Binding/type transition invariant — fixed: an existing source-bound property must be unbound before its type can change. A real concurrent bind/configure regression proves exactly one transition wins and the resulting mapping/type pair is coherent.

Verification on the repaired head: 137 targeted tests across five persistence/editor files, Content typecheck, additive-migration guard, unscoped-query guard, formatting, and git diff --check. Independent review found no remaining blocker in the unified lock protocol.

Hosted authenticated invocation, migration application, exact private-row readback, and separate-process/PostgreSQL contention remain explicit acceptance gates before unattended use.

  • Codex AI

builder-io-integration[bot]

This comment was marked as outdated.

…key-3192

# Conflicts:
#	templates/content/actions/set-document-property.ts
#	templates/content/parity/matrix.md
#	templates/content/parity/matrix.ts
builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builder reviewed your changes and found 3 potential issues 🔴

Review Details

Incremental Code Review Summary

The latest head fixes both previously reported source lifecycle issues: binding now verifies that the field/source updates affected rows, and source-role conversion deletes claims together with removed memberships. I resolved both stale threads before this pass. The exact readback-body comment remains open and was not reposted.

The remaining implementation has solid transaction-scoped cleanup for owned database deletion and comprehensive claim validation, but three new concurrency/lifecycle issues remain. Stable-key upsert currently rejects source ownership only for the key property, allowing caller writes to overwrite a different source-bound property. The unbind path still runs outside the binding transaction and can race a bind backfill. Finally, permanent deletion locks databases owned by documents in the subtree but not parent databases that own ordinary row documents, leaving a claim-orphan race for row deletion.

Risk classification: standard (shared business logic and persistent data mutation).

Key findings:

  • 🔴 HIGH — Permanent deletion does not lock parent databases for ordinary row documents.
  • 🟡 MEDIUM — Upsert can overwrite a non-key source-bound property.
  • 🟡 MEDIUM — Unbind is not serialized with bind/backfill.

🧪 Browser testing: Skipped — PR changes persistence/actions and tests, with no user-facing UI implementation changes.

Comment thread templates/content/actions/delete-document.ts
Comment thread templates/content/actions/upsert-database-item-by-key.ts Outdated
Comment thread templates/content/actions/bind-content-database-source-field.ts
@netlify

This comment has been minimized.

@3mdistal
3mdistal merged commit b5a0771 into main Aug 4, 2026
102 checks passed
@3mdistal
3mdistal deleted the codex/content-upsert-key-3192 branch August 4, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants